components/operator/internal/handlers/sessions.go (1)

1908-1911: ⚠️ Potential issue | 🟠 Major

Stop patching agentStatus from monitorPod.

Elsewhere in this change set agentStatus moved to query-time derivation from the AG-UI log, but these terminal branches still write "idle" into CR status. That recreates a stale second source of truth on the operator path and is out of step with the schema removal.

Also applies to: 1972-1975, 2015-2016

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/operator/internal/handlers/sessions.go` around lines 1908 - 1911,
In monitorPod (sessions.go) remove the writes that patch agentStatus in the Pod
terminal branches: stop calling statusPatch.SetField("agentStatus", "idle") (and
similar lines at the other terminal branches around the PodSucceeded/PodFailed
handling referenced) so the operator no longer persists agentStatus; leave other
status fields (phase, completionTime) intact and rely on the query-time
derivation from the AG-UI log instead.

components/backend/websocket/agui_store.go (1)

201-256: ⚠️ Potential issue | 🟠 Major

Avoid reparsing the whole event log on every status lookup.

DeriveAgentStatus() is now on the session list/detail read path, but each call still does os.ReadFile plus per-line JSON unmarshal of the full log. As agui-events.jsonl grows, status resolution becomes O(log size) per session. Persist a compact last-status record on write and fall back to a scan only when that cache is missing or corrupt.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/backend/websocket/agui_store.go` around lines 201 - 256,
DeriveAgentStatus currently reads and reparses agui-events.jsonl every call;
instead first attempt to read a compact cached status file (e.g.
StateBaseDir/sessions/<sessionID>/agui-status.json) and return it if valid,
falling back to the existing log scan only when the cache is missing or corrupt;
after computing status from the log, write the compact cache atomically (include
status, lastUpdated timestamp and optional runId) so future calls avoid the full
read/parse. Update the event append path that writes agui-events.jsonl (the
writer that creates tool/run events) to also update this agui-status cache when
lifecycle/tool events change status, ensuring writes are atomic and errors are
logged but do not break the main event write; keep DeriveAgentStatus’s existing
scan logic as the fallback and invalidate the cache on any write errors or
detected inconsistency.

components/runners/ambient-runner/ambient_runner/bridges/claude/bridge.py (1)

124-132: ⚠️ Potential issue | 🔴 Critical

Scope halt state to the current run, not the shared adapter.

ClaudeBridge caches a single self._adapter across thread IDs, so self._adapter.halted is shared mutable state. Another concurrent run can overwrite that flag before this check executes, which can miss the interrupt or interrupt the wrong worker. Capture the halt result from the current adapter invocation or key halt bookkeeping by thread_id.

Verify that halt state is stored per-run/thread rather than on the bridge-wide adapter instance. The concern is confirmed if self._adapter is singleton-scoped and halt bookkeeping is not keyed by thread/session.

#!/bin/bash
set -euo pipefail

echo "== Adapter lifetime =="
rg -n -C2 'self\._adapter|ClaudeAgentAdapter' components/runners/ambient-runner/ambient_runner/bridges/claude/bridge.py

echo
echo "== Halt bookkeeping =="
rg -n -C2 '\.halted\b|_halted_tool_call_id' components/runners/ambient-runner/ambient_runner

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/runners/ambient-runner/ambient_runner/bridges/claude/bridge.py`
around lines 124 - 132, The check uses the bridge-wide self._adapter.halted
which is shared across runs and can race; instead scope halt state to the
current run by reading the halted flag from the adapter instance used for this
thread or by maintaining per-thread halt bookkeeping keyed by thread_id.
Concretely: in the ClaudeBridge flow where you call into the adapter (the same
area that later checks self._adapter.halted and calls worker.interrupt), capture
the adapter returned/used for this invocation into a local variable (e.g.,
adapter = self._adapter or from whatever _get_adapter/_ensure_adapter method is
used) and use adapter.halted for the conditional, or introduce a
self._halted_by_thread dict and set/clear self._halted_by_thread[thread_id] when
the adapter reports a halt and read that keyed value before calling
worker.interrupt; update any adapter creation/teardown paths to initialize/clear
the per-thread entry.

components/frontend/src/components/ui/ask-user-question.tsx (3)

275-289: ⚠️ Potential issue | 🟡 Minor

Add explicit type="button" to prevent accidental form submission.

The tab navigation buttons lack an explicit type attribute and will default to type="submit", which can trigger unintended form submissions.

🐛 Proposed fix

                     <button
+                      type="button"
                       key={idx}
                       onClick={() => setActiveTab(idx)}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/ui/ask-user-question.tsx` around lines 275
- 289, The tab buttons currently default to submit and can trigger form
submission; update the button element (the one with onClick={() =>
setActiveTab(idx)} and rendering CheckCircle2 and {q.header || `Q${idx + 1}`})
to include an explicit type="button" attribute to prevent accidental form
submissions.

---

18-24: ⚠️ Potential issue | 🟠 Major

Await the answer send before locking the question.

The onSubmitAnswer callback is typed as synchronous void, and handleSubmit immediately sets submitted = true without awaiting. If the network send fails, the widget becomes locked with no retry path.

🐛 Proposed fix

 export type AskUserQuestionMessageProps = {
   toolUseBlock: ToolUseBlock;
   resultBlock?: ToolResultBlock;
   timestamp?: string;
-  onSubmitAnswer?: (formattedAnswer: string) => void;
+  onSubmitAnswer?: (formattedAnswer: string) => Promise<void>;
   isNewest?: boolean;
 };

+  const [isSubmitting, setIsSubmitting] = useState(false);
-  const disabled = alreadyAnswered || submitted || !isNewest;
+  const disabled = alreadyAnswered || submitted || isSubmitting || !isNewest;

-  const handleSubmit = () => {
+  const handleSubmit = async () => {
     if (!onSubmitAnswer || !allQuestionsAnswered || disabled) return;

     const answers: Record<string, string> = {};
     for (const q of questions) {
       const sel = selections[q.question];
       answers[q.question] = Array.isArray(sel) ? sel.join(", ") : (sel as string);
     }

     const response = JSON.stringify({ questions, answers });
-    onSubmitAnswer(response);
-    setSubmitted(true);
+    try {
+      setIsSubmitting(true);
+      await onSubmitAnswer(response);
+      setSubmitted(true);
+    } finally {
+      setIsSubmitting(false);
+    }
   };

Also applies to: 108-122

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/ui/ask-user-question.tsx` around lines 18
- 24, The submit flow marks the question locked immediately because
onSubmitAnswer is typed as synchronous void and handleSubmit sets submitted =
true without awaiting; update AskUserQuestionMessageProps so onSubmitAnswer
returns Promise<void> (or Promise<boolean> if you want success info), change
handleSubmit to await onSubmitAnswer(...) and only set submitted = true after
the awaited call succeeds, and add a try/catch around the await to
handle/rethrow/log errors and keep the widget unlocked on failure so the user
can retry; locate these changes in the AskUserQuestionMessageProps type and the
handleSubmit function within ask-user-question.tsx.

---

202-230: ⚠️ Potential issue | 🟡 Minor

Add keyboard support to the "Other" option for accessibility.

The "Other" option uses onClick without corresponding keyboard event handlers (onKeyDown/onKeyUp), making it inaccessible to keyboard-only users. This violates a11y requirements flagged by Biome.

♿ Proposed fix

         <label
           className={cn(
             "flex items-center gap-2.5 p-1.5 rounded cursor-pointer transition-colors",
             isOther ? "bg-accent" : "hover:bg-muted/50",
             disabled && "cursor-default opacity-60"
           )}
           onClick={() => !disabled && handleOtherToggle(q.question)}
+          onKeyDown={(e) => {
+            if (!disabled && (e.key === "Enter" || e.key === " ")) {
+              e.preventDefault();
+              handleOtherToggle(q.question);
+            }
+          }}
+          tabIndex={disabled ? -1 : 0}
+          role="button"
         >

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/ui/ask-user-question.tsx` around lines 202
- 230, The "Other" label currently only handles mouse clicks via onClick and is
not keyboard-accessible; update the label wrapper to be keyboard-focusable (add
tabIndex={0} and role="button") and add an onKeyDown handler that calls
handleOtherToggle(q.question) when Enter or Space is pressed (respecting the
disabled flag), ensuring the same disabled-check used in the existing onClick is
applied; keep the existing Input onClick stopPropagation and ensure the added
onKeyDown does not interfere with focus when the inner Input is rendered.

components/frontend/src/components/session/MessagesTab.tsx (1)

143-152: ⚠️ Potential issue | 🟠 Major

Use stable IDs instead of array indices for message keys to preserve component state.

When showSystemMessages toggles, the filteredMessages array changes shape. Index-based keys (sm-${idx}) cause StreamMessage components to remount, which will reset AskUserQuestionMessage local state (selections, tabs, freeform input).

🐛 Proposed fix

-        {shouldShowMessages && filteredMessages.map((m, idx) => (
-          <StreamMessage
-            key={`sm-${idx}`}
-            message={m}
-            isNewest={idx === filteredMessages.length - 1}
-            onGoToResults={onGoToResults}
-            agentName={agentName}
-            onSubmitAnswer={onSendToolAnswer}
-          />
-        ))}
+        {shouldShowMessages && filteredMessages.map((m, idx) => {
+          const messageKey =
+            "toolUseBlock" in m ? `tool-${m.toolUseBlock.id}` : `msg-${m.id ?? idx}`;
+          return (
+            <StreamMessage
+              key={messageKey}
+              message={m}
+              isNewest={idx === filteredMessages.length - 1}
+              onGoToResults={onGoToResults}
+              agentName={agentName}
+              onSubmitAnswer={onSendToolAnswer}
+            />
+          );
+        })}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/session/MessagesTab.tsx` around lines 143
- 152, The keys for StreamMessage are unstable because they use array indices
(sm-${idx}); update MessagesTab to use a stable unique identifier from each
message object (e.g., m.id or another persistent field like
m.messageId/timestamp) as the key so StreamMessage instances (and nested
AskUserQuestionMessage state) are preserved across filteredMessages shape
changes; locate the JSX mapping in MessagesTab where filteredMessages.map
produces StreamMessage and replace the key generation to reference the message's
stable id field, ensuring all messages have that id or fallback to a stable
composite key derived from message properties.

components/backend/handlers/sessions.go (1)

367-384: ⚠️ Potential issue | 🟠 Major

Missing namespace in agent status derivation call.

This function extracts only the session name but not the namespace. To fix the cross-project collision issue, extract and pass the namespace:

🔧 Suggested fix

 func enrichAgentStatus(session *types.AgenticSession) {
 	if session.Status == nil || session.Status.Phase != "Running" {
 		return
 	}
 	if DeriveAgentStatusFromEvents == nil {
 		return
 	}
 	name, _ := session.Metadata["name"].(string)
+	namespace, _ := session.Metadata["namespace"].(string)
-	if name == "" {
+	if name == "" || namespace == "" {
 		return
 	}
-	if derived := DeriveAgentStatusFromEvents(name); derived != "" {
+	if derived := DeriveAgentStatusFromEvents(namespace, name); derived != "" {
 		session.Status.AgentStatus = types.StringPtr(derived)
 	}
 }

This requires updating DeriveAgentStatusFromEvents signature and the underlying websocket.DeriveAgentStatus implementation to accept namespace.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/backend/handlers/sessions.go` around lines 367 - 384,
enrichAgentStatus currently only extracts the session name and calls
DeriveAgentStatusFromEvents(name), causing cross-project collisions; update
enrichAgentStatus (in components/backend/handlers/sessions.go) to also extract
namespace from session.Metadata["namespace"] (string), validate it, and call the
updated DeriveAgentStatusFromEvents(namespace, name) signature; update the
global function type/signature for DeriveAgentStatusFromEvents and its
implementation websocket.DeriveAgentStatus to accept (namespace, name) and
propagate the namespace change to any other callers so derived AgentStatus is
namespaced.

components/frontend/src/components/session/ask-user-question.tsx (1)

18-24: ⚠️ Potential issue | 🟠 Major

Type mismatch: onSubmitAnswer should return Promise<void> to match upstream.

The prop is typed as (formattedAnswer: string) => void, but MessagesTabProps.onSendToolAnswer is typed as (formattedAnswer: string) => Promise<void>. This mismatch prevents proper async error handling in handleSubmit.

Suggested fix

 export type AskUserQuestionMessageProps = {
   toolUseBlock: ToolUseBlock;
   resultBlock?: ToolResultBlock;
   timestamp?: string;
-  onSubmitAnswer?: (formattedAnswer: string) => void;
+  onSubmitAnswer?: (formattedAnswer: string) => Promise<void>;
   isNewest?: boolean;
 };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/session/ask-user-question.tsx` around
lines 18 - 24, The prop type onSubmitAnswer in AskUserQuestionMessageProps is
currently synchronous but must be async to match
MessagesTabProps.onSendToolAnswer; change the type of onSubmitAnswer to
(formattedAnswer: string) => Promise<void>, update any usages (e.g., the
handleSubmit function inside this component) to await the call and
return/propagate the Promise so errors can be handled upstream, and ensure
callers (who implement MessagesTabProps.onSendToolAnswer) remain compatible with
the new signature; reference AskUserQuestionMessageProps, onSubmitAnswer,
handleSubmit, and MessagesTabProps.onSendToolAnswer when making the change.

components/backend/websocket/agui_store.go (1)

198-248: ⚠️ Potential issue | 🟡 Minor

Legacy session migration not triggered — status will be empty until event replay.

DeriveAgentStatus reads agui-events.jsonl directly (line 203) but does not call MigrateLegacySessionToAGUI like loadEvents does (lines 170-173). For legacy sessions that only have messages.jsonl, this function will return "" until the user opens /agui/events (which triggers migration via loadEvents).

Consider either:

1. Calling migration before the file read, or
2. Documenting this as intentional behavior (migration happens on first event replay)

🔧 Proposed fix to trigger migration

 func DeriveAgentStatus(sessionID string) string {
 	path := fmt.Sprintf("%s/sessions/%s/agui-events.jsonl", StateBaseDir, sessionID)
 
+	// Trigger migration for legacy sessions (messages.jsonl → agui-events.jsonl)
+	if _, err := os.Stat(path); os.IsNotExist(err) {
+		_ = MigrateLegacySessionToAGUI(sessionID)
+	}
+
 	// Read only the tail of the file to avoid loading entire event log into memory.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/backend/websocket/agui_store.go` around lines 198 - 248,
DeriveAgentStatus reads agui-events.jsonl directly and therefore returns empty
for legacy sessions that only have messages.jsonl; call
MigrateLegacySessionToAGUI(sessionID) at the start of DeriveAgentStatus (before
opening/reading the file) to ensure legacy sessions are migrated to
agui-events.jsonl—mirror what loadEvents does—so the event log exists for status
derivation; keep existing error handling if migration fails.

components/runners/ambient-runner/ambient_runner/bridges/claude/bridge.py (1)

127-132: ⚠️ Potential issue | 🔴 Critical

Don’t read halt state from the shared adapter here.

Line 127 still gates the interrupt on self._adapter.halted, but components/runners/ambient-runner/ag_ui_claude_sdk/adapter.py:237-250 exposes that as a single _halted flag and components/runners/ambient-runner/ag_ui_claude_sdk/adapter.py:910-920 mutates it during a run. Since ClaudeBridge reuses one adapter across thread-scoped workers, concurrent runs can overwrite that flag before this branch executes, so you can miss the current halt or interrupt the wrong worker. Capture the halt outcome from the specific run() invocation, or key halt state by thread_id instead of reading bridge-wide adapter state here.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/runners/ambient-runner/ambient_runner/bridges/claude/bridge.py`
around lines 127 - 132, The interrupt check in ClaudeBridge currently reads the
bridge-wide flag self._adapter.halted which can be overwritten by concurrent
runs; instead capture the halt outcome for this specific run (e.g., return a
halted boolean from adapter.run(...) or have adapter.record_halt(thread_id) and
read that per-thread key) and use that value to decide whether to call
worker.interrupt(); update the ClaudeBridge logic (the branch checking
self._adapter.halted before awaiting worker.interrupt()) to use the run-specific
halted result or per-thread halted map keyed by thread_id so concurrent runs
cannot clobber each other.

Makefile (1)

737-740: ⚠️ Potential issue | 🟠 Major

Still unresolved: avoid kill 0 and the hard-coded namespace here.

kill 0 can tear down the whole recipe process group, including the enclosing make, and -n ambient-code makes NAMESPACE=... make kind-port-forward forward the wrong services. Capture the two kubectl PIDs, kill those explicitly in the trap, and use $(NAMESPACE) for both commands.

Proposed fix

-	`@trap` 'kill 0; echo ""; echo "$(COLOR_GREEN)✓$(COLOR_RESET) Port forwarding stopped"; exit 0' INT; \
-	kubectl port-forward -n ambient-code svc/frontend-service $(KIND_FWD_FRONTEND_PORT):3000 >/dev/null 2>&1 & \
-	kubectl port-forward -n ambient-code svc/backend-service $(KIND_FWD_BACKEND_PORT):8080 >/dev/null 2>&1 & \
-	wait
+	`@frontend_pid`=; backend_pid=; \
+	trap 'kill $$frontend_pid $$backend_pid 2>/dev/null || true; echo ""; echo "$(COLOR_GREEN)✓$(COLOR_RESET) Port forwarding stopped"; exit 0' INT; \
+	kubectl port-forward -n $(NAMESPACE) svc/frontend-service $(KIND_FWD_FRONTEND_PORT):3000 >/dev/null 2>&1 & frontend_pid=$$!; \
+	kubectl port-forward -n $(NAMESPACE) svc/backend-service $(KIND_FWD_BACKEND_PORT):8080 >/dev/null 2>&1 & backend_pid=$$!; \
+	wait $$frontend_pid $$backend_pid

Run this read-only check to confirm the target still contains both problems and that NAMESPACE is configurable elsewhere:

#!/bin/bash
sed -n '729,740p' Makefile
echo '---'
rg -n '^[[:space:]]*NAMESPACE[[:space:]]*\?=|kill 0|port-forward -n ambient-code' Makefile

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Makefile` around lines 737 - 740, The trap currently uses `kill 0` and the
port-forward lines hard-code `ambient-code`; change the recipe to capture the
two background kubectl PIDs (e.g., start each `kubectl port-forward -n
$(NAMESPACE) svc/... ... &` and save their $$! values into variables like
FRONTEND_PID and BACKEND_PID), update both `-n ambient-code` occurrences to `-n
$(NAMESPACE)`, and in the trap kill those specific PIDs (kill -TERM
$(FRONTEND_PID) $(BACKEND_PID) || true) and wait for them instead of `kill 0`;
keep the existing redirection and `wait` behavior so the foreground Make job
still blocks until both forwards exit.

components/runners/ambient-runner/ag_ui_claude_sdk/adapter.py (1)

919-923: ⚠️ Potential issue | 🔴 Critical

Duplicate TOOL_CALL_END emitted after frontend tool halt.

After setting the halt state and continue, the loop breaks at line 680, entering the finally block. Since current_tool_call_id is not cleared, line 1085 emits a second ToolCallEndEvent for the same tool call.

This violates the AG-UI tool call sequence: TOOL_CALL_START → TOOL_CALL_ARGS → TOOL_CALL_END (exactly once per tool call).

,

🐛 Proposed fix: Clear tool call state after setting halt flags

                                self._halted = True
                                self._halted_tool_call_id = current_tool_call_id
                                halt_event_stream = True
+
+                                # Clear tool state to avoid duplicate TOOL_CALL_END in finally block
+                                current_tool_call_id = None
+                                current_tool_call_name = None
+                                current_tool_display_name = None
+                                accumulated_tool_json = ""
+
                                 # Continue consuming remaining events for cleanup
                                 continue

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/runners/ambient-runner/ag_ui_claude_sdk/adapter.py` around lines
919 - 923, When you set self._halted = True and self._halted_tool_call_id =
current_tool_call_id inside the loop, also clear the in-progress tool-call state
(e.g., set current_tool_call_id = None or clear the attribute that tracks the
active tool call) before using continue so that the finally block in the
surrounding function does not emit a second ToolCallEndEvent; update the same
block where you set self._halted/self._halted_tool_call_id and ensure the
finally clause that emits ToolCallEndEvent checks that current_tool_call_id (or
the instance attribute tracking the active tool call) is not None and matches
expected state before emitting.

components/frontend/src/components/session/ask-user-question.tsx (2)

278-292: ⚠️ Potential issue | 🟡 Minor

Add type="button" to tab navigation buttons.

These buttons lack an explicit type attribute and will default to type="submit", causing unintended form submissions if this component is used inside a form.

🔧 Proposed fix

                     <button
+                      type="button"
                       key={idx}
                       onClick={() => setActiveTab(idx)}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/session/ask-user-question.tsx` around
lines 278 - 292, The tab buttons in the render loop (the <button> that calls
setActiveTab(idx) and conditionally renders CheckCircle2 and
q.header/`Q${idx+1}`) are missing an explicit type and will default to submit
inside forms; update that JSX button element to include type="button" so clicks
do not trigger form submission, leaving the onClick, className logic, and
conditional children unchanged.

---

111-125: ⚠️ Potential issue | 🟠 Major

Await the answer submission before marking as submitted.

handleSubmit calls onSubmitAnswer and immediately sets submitted = true. If the network send fails, the question UI remains locked with no retry path. Make onSubmitAnswer return Promise<void>, add loading state, and only mark submitted on success.

🔧 Proposed fix

 export type AskUserQuestionMessageProps = {
   toolUseBlock: ToolUseBlock;
   resultBlock?: ToolResultBlock;
   timestamp?: string;
-  onSubmitAnswer?: (formattedAnswer: string) => void;
+  onSubmitAnswer?: (formattedAnswer: string) => Promise<void>;
   isNewest?: boolean;
 };

   const [submitted, setSubmitted] = useState(false);
-  const disabled = alreadyAnswered || submitted || !isNewest;
+  const [isSubmitting, setIsSubmitting] = useState(false);
+  const disabled = alreadyAnswered || submitted || isSubmitting || !isNewest;

-  const handleSubmit = () => {
+  const handleSubmit = async () => {
     if (!onSubmitAnswer || !allQuestionsAnswered || disabled) return;

     const answers: Record<string, string> = {};
     for (const q of questions) {
       const sel = selections[q.question];
       answers[q.question] = Array.isArray(sel) ? sel.join(", ") : (sel as string);
     }

     const response = JSON.stringify({ questions, answers });
-    onSubmitAnswer(response);
-    setSubmitted(true);
+    try {
+      setIsSubmitting(true);
+      await onSubmitAnswer(response);
+      setSubmitted(true);
+    } finally {
+      setIsSubmitting(false);
+    }
   };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/session/ask-user-question.tsx` around
lines 111 - 125, The handleSubmit function currently calls onSubmitAnswer and
immediately calls setSubmitted(true); instead change it to await the async
submission and only mark submitted on success: update the onSubmitAnswer
signature to return Promise<void>, add a local loading state (e.g.,
isSubmitting) toggled true before calling onSubmitAnswer and false in finally,
await onSubmitAnswer(response) inside handleSubmit, and only call
setSubmitted(true) after the awaited call resolves; also handle errors by not
setting submitted and optionally surfacing the error so the UI can allow
retries; key symbols: handleSubmit, onSubmitAnswer, setSubmitted.

components/backend/handlers/sessions.go (3)

673-676: ⚠️ Potential issue | 🟠 Major

Millisecond timestamps still allow duplicate session names.

Two create requests in the same millisecond still produce the same CR name here, and the Create() failure below is surfaced as a 500. Use Kubernetes generateName or retry on AlreadyExists so burst session creation does not fail spuriously.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/backend/handlers/sessions.go` around lines 673 - 676, The current
session name generation (timestamp := time.Now().UnixMilli(); name :=
fmt.Sprintf("session-%d", timestamp)) can collide on burst creates; update the
session creation to use Kubernetes metadata GenerateName (e.g. set
ObjectMeta.GenerateName = "session-" so the API will append a unique suffix) or
implement a retry around the Create() call that specifically catches
IsAlreadyExists errors and retries with a new name (or backoff) until success;
modify the code paths that use the variables name/timestamp and the Create()
invocation to adopt GenerateName or the retry-on-AlreadyExists strategy.

---

439-440: ⚠️ Potential issue | 🟠 Major

Derive agent status after pagination, not before.

enrichAgentStatus() now runs while iterating the full namespace result set, even though this handler only returns paginatedSessions. Because derivation hits the event-log path, list latency scales with all running sessions instead of the page size. Enrich only the paginated slice after filtering/sorting/pagination.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/backend/handlers/sessions.go` around lines 439 - 440,
enrichAgentStatus is being called for every session in the full namespace result
set, causing event-log lookups for all running sessions; move the call so it
only runs on the final paginated slice. After you perform filtering, sorting,
and pagination to produce paginatedSessions, iterate over that slice and call
enrichAgentStatus(&session) (or the equivalent loop that currently calls
enrichAgentStatus) only for those sessions to avoid scaling list latency with
the total namespace size.

---

44-46: ⚠️ Potential issue | 🔴 Critical

Namespace is still missing from the event-store lookup key.

DeriveAgentStatusFromEvents only accepts a bare session identifier, and the current callers pass the session name. Since the AG-UI store is also keyed by that value, two projects that create the same session name can read/write the same event log and derive the wrong status. Make the store key namespace-qualified end to end, or switch the store to use the CR UID.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/backend/handlers/sessions.go` around lines 44 - 46,
DeriveAgentStatusFromEvents currently accepts only a bare session identifier
(sessionID) which allows collisions across projects because callers pass session
name; update the function signature and all callers to include a
namespace-qualified key (e.g., namespace + "/" + sessionName) or switch to using
the unique CR UID instead of the name so the event-store lookup key is globally
unique; locate and update the function DeriveAgentStatusFromEvents and any
callers in the websocket/AG-UI integration to pass the new namespaced key (or CR
UID), and ensure the event-store read/write logic uses that same namespaced/UID
key end-to-end to prevent cross-project event mixing.

components/backend/websocket/agui_store.go (1)

202-248: ⚠️ Potential issue | 🟠 Major

Route status derivation through the canonical event loader.

This reads agui-events.jsonl directly and bypasses loadEvents() at Lines 164-195, which is the only path here that performs the legacy messages.jsonl → agui-events.jsonl migration. Older sessions will therefore keep returning "" for status until some other endpoint happens to trigger migration. Reuse the same loader/migration path as replay.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/backend/websocket/agui_store.go` around lines 202 - 248,
DeriveAgentStatus currently reads agui-events.jsonl directly and bypasses the
migration done by loadEvents (the loader that handles messages.jsonl →
agui-events.jsonl); change DeriveAgentStatus to call loadEvents(sessionID) to
obtain the canonical event list (and let loadEvents perform migration if
needed), then iterate the returned events to compute and return the same status
string instead of manually opening agui-events.jsonl; ensure you handle
nil/empty/err from loadEvents the same way the existing callers expect
(returning "" on failure).

components/frontend/src/services/queries/use-sessions.ts (1)

56-60: ⚠️ Potential issue | 🟠 Major

Treat missing agentStatus as active for the 5s polling tier.

Line 58 only matches agentStatus === 'working'. agentStatus is optional, so newly started Running sessions that haven't derived a status yet still drop into the 15s bucket and the list can stay stale for an extra cycle. Keep missing/empty agentStatus in the 5s path as well.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/services/queries/use-sessions.ts` around lines 56 -
60, The current 5s polling tier predicate (the hasWorking = items.some(...)
check) only treats sessions with s.status?.agentStatus === 'working' as active;
change it to also treat a missing/empty agentStatus as active so newly started
Running sessions are included. Update the items.some predicate used to compute
hasWorking to return true when s.status?.phase === 'Running' and
(s.status?.agentStatus === 'working' OR s.status?.agentStatus is
null/undefined/empty). Keep the variable name hasWorking and the early return
(if (hasWorking) return 5000) unchanged.

components/frontend/src/components/ui/stream-message.tsx (1)

13-20: ⚠️ Potential issue | 🟠 Major

Type mismatch: onSubmitAnswer should return Promise<void>.

StreamMessageProps.onSubmitAnswer is typed as (formattedAnswer: string) => void, but the actual handler (sendToolAnswer in page.tsx) returns Promise<void>. This type inconsistency silently discards the Promise, preventing proper error handling in the component.

🐛 Proposed fix

 export type StreamMessageProps = {
   message: (MessageObject | ToolUseMessages | HierarchicalToolMessage) & { streaming?: boolean };
   onGoToResults?: () => void;
-  onSubmitAnswer?: (formattedAnswer: string) => void;
+  onSubmitAnswer?: (formattedAnswer: string) => Promise<void>;
   plainCard?: boolean;
   isNewest?: boolean;
   agentName?: string;
 };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/ui/stream-message.tsx` around lines 13 -
20, StreamMessageProps has onSubmitAnswer typed as (formattedAnswer: string) =>
void but the actual handler sendToolAnswer returns Promise<void>; update the
prop signature in StreamMessageProps to onSubmitAnswer?: (formattedAnswer:
string) => Promise<void> so callers can await it and surface errors; update any
usages (e.g., the StreamMessage component prop and where sendToolAnswer is
passed) to treat it as an async function and await or handle its Promise
accordingly.

components/frontend/src/app/projects/[name]/sessions/[sessionName]/page.tsx (1)

1401-1410: ⚠️ Potential issue | 🟡 Minor

Consider re-throwing errors from sendToolAnswer to allow UI components to handle failures.

The sendToolAnswer function catches errors and shows a toast but doesn't re-throw. This prevents the AskUserQuestionMessage component from knowing if the send failed, potentially leaving the UI in a submitted state when the answer wasn't actually sent.

🛡️ Proposed fix

   const sendToolAnswer = async (formattedAnswer: string) => {
     try {
       await aguiSendMessage(formattedAnswer, {
         type: "ask_user_question_response",
       });
     } catch (err) {
       toast.error(err instanceof Error ? err.message : "Failed to send answer");
+      throw err;
     }
   };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/app/projects/`[name]/sessions/[sessionName]/page.tsx
around lines 1401 - 1410, The sendToolAnswer function currently swallows errors
(catching them and showing a toast) so callers like AskUserQuestionMessage can't
detect failures; update sendToolAnswer (the function that calls aguiSendMessage)
to re-throw the caught error after showing the toast (or alternatively return a
rejected Promise/explicit failure value) so the AskUserQuestionMessage component
can handle the failure state; ensure you reference sendToolAnswer and
aguiSendMessage in the change and propagate the error back to the caller instead
of silently absorbing it.

components/frontend/src/components/agent-status-indicator.tsx (1)

25-105: 🧹 Nitpick | 🔵 Trivial

Consider adding exhaustive switch handling.

The switch statement covers all current AgentStatus values but lacks a default case. If new status values are added to AgentStatus in the future, this component will silently return undefined.

♻️ Suggested enhancement for compile-time safety

     case "idle":
       return (
         <div className={cn("flex items-center gap-1.5", className)}>
           <Circle
             className={cn(
               "text-gray-400",
               compact ? "w-3.5 h-3.5" : "w-4 h-4"
             )}
           />
           {!compact && (
             <span className="text-sm text-muted-foreground">Idle</span>
           )}
         </div>
       );
+
+    default: {
+      const _exhaustiveCheck: never = status;
+      return _exhaustiveCheck;
+    }
   }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/components/agent-status-indicator.tsx` around lines
25 - 105, The switch in the AgentStatusIndicator component (the switch(status)
handling "working", "waiting_input", "completed", "failed", "idle") is not
exhaustive and can return undefined if AgentStatus gains new values; add an
explicit default branch to enforce exhaustiveness — either call an
assertUnreachable(value) helper (throwing an error) to surface new statuses at
runtime/compile-time or return a safe fallback UI (e.g., null or a generic
Badge) so the component never returns undefined; implement the chosen approach
in the switch's default case and/or add an assertUnreachable function to the
module.

components/backend/handlers/sessions.go (2)

677-680: ⚠️ Potential issue | 🟠 Major

UnixMilli() still leaves a collision path here.

Two creates in the same namespace within one millisecond will generate the same CR name, and Create() still turns the resulting AlreadyExists into a 500. Prefer Kubernetes generateName and return created.GetName() / ComputeAutoBranch(created.GetName()), or retry on errors.IsAlreadyExists.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/backend/handlers/sessions.go` around lines 677 - 680, The current
session name generation (timestamp := time.Now().UnixMilli(); name :=
fmt.Sprintf("session-%d", timestamp)) can collide if two creates occur in the
same millisecond; change the creation logic to use Kubernetes server-side name
generation by setting the object metadata GenerateName (e.g. "session-") instead
of precomputing name, call Create(), then return created.GetName() /
ComputeAutoBranch(created.GetName()); alternatively implement a small retry loop
around the client Create() that catches errors.IsAlreadyExists and retries a few
times before failing—update the code paths that reference name, Create(),
ComputeAutoBranch to use the created object's actual name.

---

443-445: ⚠️ Potential issue | 🟠 Major

Derive agentStatus only for the returned page.

This still hits the event log for every session before search/pagination, even though only paginatedSessions are returned. On larger namespaces, list latency now scales with the full result set instead of the page size.

♻️ Suggested change

-		// Derive agentStatus from event log (source of truth) for running sessions
-		enrichAgentStatus(&session)
-
 		session.AutoBranch = ComputeAutoBranch(item.GetName())
 
 		sessions = append(sessions, session)
 	}
@@
 	// Apply pagination
 	totalCount := len(sessions)
 	paginatedSessions, hasMore, nextOffset := paginateSessions(sessions, params.Offset, params.Limit)
+	for i := range paginatedSessions {
+		enrichAgentStatus(&paginatedSessions[i])
+	}

Also applies to: 459-461

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/backend/handlers/sessions.go` around lines 443 - 445, The code
currently calls enrichAgentStatus(&session) for every session in the full result
set, causing event-log lookups to scale with the full dataset; change this so
agent status is derived only for the sessions being returned by moving the
enrichAgentStatus calls to after pagination and running it only over the
paginatedSessions slice (i.e., replace the existing per-session
enrichAgentStatus calls before search/pagination with a loop that calls
enrichAgentStatus(&s) for each s in paginatedSessions). Apply the same change
for the other occurrence referenced (the block around enrichAgentStatus at the
second location) so both places derive agentStatus only for the returned page.

components/backend/websocket/agui_store.go (1)

203-213: ⚠️ Potential issue | 🟠 Major

Don't bypass the legacy migration path here.

This opens agui-events.jsonl directly and returns "" when it is missing, but loadEvents() is the path that migrates legacy messages.jsonl sessions. Until another endpoint triggers that migration, older sessions lose derived agentStatus in list/detail views.

♻️ Suggested change

 	file, err := os.Open(path)
-	if err != nil {
+	if err != nil && os.IsNotExist(err) {
+		if mErr := MigrateLegacySessionToAGUI(sessionID); mErr == nil {
+			file, err = os.Open(path)
+		}
+	}
+	if err != nil {
 		return ""
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/backend/websocket/agui_store.go` around lines 203 - 213,
DeriveAgentStatus currently opens agui-events.jsonl directly and returns empty
on missing files, skipping the legacy migration handled by loadEvents; change
DeriveAgentStatus (and its use of StateBaseDir/agui-events.jsonl) to call or
delegate to loadEvents for the session when agui-events.jsonl is not present so
the legacy messages.jsonl migration runs and agentStatus can be derived;
specifically, detect the missing-file case in DeriveAgentStatus, invoke
loadEvents(sessionID) to trigger migration/creation of agui-events.jsonl (or
reuse its returned event stream) and then proceed to compute the status instead
of returning "".